fix(logger): add Pino PagerDuty V2 transport support#4914
Merged
Reinis-FRP merged 20 commits intomasterfrom Jan 26, 2026
Merged
fix(logger): add Pino PagerDuty V2 transport support#4914Reinis-FRP merged 20 commits intomasterfrom
Reinis-FRP merged 20 commits intomasterfrom
Conversation
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
- Create shared PagerDuty config module (types, validation, level conversion) - Implement PagerDutyV2Transport for Pino using pino-abstract-transport - Refactor Winston PagerDutyV2Transport to use shared config - Add createPinoTransports with conditional PagerDuty integration - Fix log level handling (read from env/config instead of hardcoded) - Update createPinoLogger to pass level to transports - Export Transports from logger package index - Match Winston's fail-fast behavior for config validation WIP: needs review and testing before finalizing Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Change from inline type imports (import { type Foo }) to separate
import type statements to match project's linter configuration.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Remove re-exports of Config, createConfig, and types from transport files. These were only used internally and are now imported directly from the SharedConfig module by consumers (Transports.ts files). This makes it clear that these are shared utilities, not transport-specific APIs, and prevents them from being inadvertently exposed in the public API. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
…sConfig The environment parameter was copied from Winston's interface but is not needed for Pino. Unlike Winston, Pino doesn't require different transports for different environments - it always outputs JSON to stdout which GCP automatically parses. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
The previous implementation created a local variable that was never used, so the unmodified mrkdwn was being sent to PagerDuty. Now it directly mutates obj.mrkdwn to match the Winston implementation. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Extract the PagerDuty event() call to a shared sendPagerDutyEvent() helper that accepts the whole log object and routing key. The helper now handles field extraction (level, message, at, botIdentifier) with fallbacks for both Winston and Pino log formats. This eliminates duplication and simplifies both transports - they now just call sendPagerDutyEvent(routing_key, logObj) instead of manually extracting and passing individual fields. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Move the removeAnchorTextFromLinks call into the shared sendPagerDutyEvent helper. This further simplifies both transports and ensures markdown is always cleaned consistently before sending to PagerDuty. Both transports are now extremely simple - just get the routing key and call sendPagerDutyEvent(routing_key, logObj). All field extraction, markdown cleaning, and event formatting is handled by the shared helper. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
…nsport.ts Reorganize PagerDuty shared utilities from pagerduty/SharedConfig.ts to shared/PagerDutyV2Transport.ts. This structure: - Matches naming convention of the transport files - Allows shared/ directory to contain other transport utilities - Eliminates the single-file pagerduty/ directory - Clearly indicates these are shared PagerDuty V2 utilities Updated all imports from ../pagerduty/SharedConfig to ../shared/PagerDutyV2Transport. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Remove unnecessary intermediate variables and fallbacks from sendPagerDutyEvent. Directly use logObj fields in payload construction to match original Winston implementation behavior. This preserves the original behavior where missing required fields appear as "undefined" in PagerDuty, making bugs in log formatting obvious. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
- Remove parse: "lines" option which was causing logs to be passed as strings instead of parsed objects
- Use Pino's levels.labels to convert numeric levels (50) to strings ("error") in summary
- Always log transport errors to console in Pino (no callback mechanism like Winston)
This fixes the issue where Pino logs were not being sent to PagerDuty due to incorrect parsing.
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Configure Pino with stdSerializers.err to properly serialize Error objects with type, message, and stack properties. This matches Winston's errorStackTracerFormatter behavior and ensures errors are logged correctly instead of appearing as empty objects. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Replace manual numeric range checks with Pino's levels.labels mapping to convert numeric levels to strings. This is cleaner, more maintainable, and reuses Pino's existing infrastructure. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Fix convertLevelToSeverity to map unmapped levels (debug, trace) to "info" (lowest PagerDuty severity) instead of "error" (high severity). This bug was previously hidden because PagerDuty transports filter at level: "error", so debug/trace logs never reached the function in practice. This ensures correct severity mapping if transport level filtering is changed. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Add unit tests for: - Shared utilities (createConfig, convertLevelToSeverity) - Winston PagerDutyV2Transport (routing keys, error handling, callbacks) - Pino PagerDutyV2Transport (stream processing, routing keys, error handling) Tests verify correct behavior for both Winston string levels and Pino numeric levels, custom routing keys via notificationPath, and proper error handling. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Revert discord-ticket-api server.ts changes to master version. These changes will be submitted in a separate PR. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
c0cad53 to
9f838c1
Compare
5 tasks
md0x
reviewed
Jan 23, 2026
| // PD v2 severity only supports critical, error, warning or info. | ||
| // Handles both Winston string levels and Pino numeric levels. | ||
| export function convertLevelToSeverity(level?: string | number): Severity { | ||
| if (!level) return "info"; |
Contributor
There was a problem hiding this comment.
I notice Winston was returning an error if if (!level), but agree it makes more sense to return info here
md0x
reviewed
Jan 23, 2026
| await sendPagerDutyEvent(routing_key, obj); | ||
| } catch (error) { | ||
| // Always log transport errors in Pino since there's no callback mechanism like Winston | ||
| console.error("PagerDuty v2 transport error:", error); |
Contributor
There was a problem hiding this comment.
Should we add more context here like:
console.error("PagerDuty v2 transport error:", error, { logObj: obj });
md0x
reviewed
Jan 23, 2026
| if (transportsConfig.pagerDutyV2Config || process.env.PAGER_DUTY_V2_CONFIG) { | ||
| // to disable pdv2, pass in a "disabled=true" in configs or env. | ||
| const { disabled = false, ...pagerDutyV2Config } = | ||
| transportsConfig.pagerDutyV2Config ?? JSON.parse(process.env.PAGER_DUTY_V2_CONFIG || "null"); |
Contributor
There was a problem hiding this comment.
if PAGER_DUTY_V2_CONFIG contains an invalid json this will throw an unhandled error. Should we try catch this and so we have a more descriptive error message?
Contributor
There was a problem hiding this comment.
Nit: Eventually adding a test with an invalid config could be nice.
Contributor
Author
- Add log object context to Pino transport error messages for better debugging - Add comprehensive error handling for PAGER_DUTY_V2_CONFIG JSON parsing - Use isRecordStringUnknown from @uma/common to validate parsed config is a proper object - Provide clear error messages for invalid JSON syntax and non-object values Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
Add comprehensive tests for createPinoTransports error handling: - Invalid JSON syntax (malformed JSON) - Non-object JSON values (array, null, string, number) - Valid config with disabled flag Addresses PR review feedback requesting test coverage for invalid configs. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The new
discord-ticket-apipackage uses Fastify with Pino logger. The existing PagerDuty V2 transport only supported Winston, so we needed to add Pino support to maintain consistent PagerDuty alerting across all services that use the@uma/loggerpackage.Summary
pino-abstract-transportshared/PagerDutyV2Transport.tsto eliminate code duplication between Winston and Pino implementationsstdSerializers.err) to Pino logger config for proper Error object serializationDetails
Implementation:
pinoLogger/PagerDutyV2Transport.ts): Stream-based transport running in worker thread, processes newline-delimited JSON logsshared/PagerDutyV2Transport.ts): ContainsConfigtype,createConfigvalidation,convertLevelToSeveritymapping, andsendPagerDutyEventhelperlogger/PagerDutyV2Transport.ts): Updated to use shared utilities, no functional changes to production behaviorKey technical decisions:
levels.labelsmapping to convert numeric levels (50 = error, 40 = warn, etc.) to string names"info"(lowest PagerDuty severity) instead of"error"to maintain semantic correctnesslevel: "error"so only error and fatal logs reach PagerDutyFiles modified:
packages/logger/src/pinoLogger/PagerDutyV2Transport.ts- New Pino transportpackages/logger/src/pinoLogger/Logger.ts- Added error serializerspackages/logger/src/pinoLogger/Transports.ts- Added PagerDuty configpackages/logger/src/shared/PagerDutyV2Transport.ts- New shared utilities modulepackages/logger/src/logger/PagerDutyV2Transport.ts- Refactored to use shared utilitiesTesting
Issue(s)
Part of the work to support Pino logger in new services.